dash-spv/src/sync/block_headers/sync_manager.rs (1)

155-156: Consider potential I/O on every peer update.

The target height is persisted on every PeersUpdated event with a new best_height. While this ensures durability, it may result in frequent disk writes during peer discovery. This is acceptable for correctness since atomic writes are used, but consider adding a note or verifying that the write frequency is acceptable in high-churn scenarios.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@dash-spv/src/sync/block_headers/sync_manager.rs` around lines 155 - 156, The
code persists target height on every PeersUpdated event which may cause
excessive I/O; modify the logic around metadata_storage.store_last_target_height
to avoid writing on every update — for example, read the current persisted
height (via metadata_storage) and only call store_last_target_height when
*best_height* is greater than the stored value or when a configured
debounce/interval has elapsed; locate this in sync_manager.rs where PeersUpdated
is handled and update the branch that references
metadata_storage.store_last_target_height to perform the check or debounce
before persisting.

Configuration used: defaults

Review profile: CHILL

Plan: Pro

dash-spv/src/storage/metadata.rs (1)

103-107: Consider preserving the original error instead of mapping to Corruption.

The Err(e) case wraps any load_metadata failure as StorageError::Corruption, but the underlying error could be an I/O error, permission issue, or other non-corruption failure. This loses error context and may mislead debugging efforts.

♻️ Suggested improvement

             Err(e) => {
-                let error = format!("Failed to load last target height: {}", e);
-                tracing::warn!(error);
-                Err(StorageError::Corruption(error))
+                tracing::warn!("Failed to load last target height: {}", e);
+                Err(e)
             }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@dash-spv/src/storage/metadata.rs` around lines 103 - 107, The current Err(e)
arm wraps any failure as StorageError::Corruption (see the Err(e) block that
formats "Failed to load last target height") which loses the original error
context; change it to preserve or propagate the original error (e.g., return the
underlying error, wrap it in a more appropriate StorageError variant like Io or
include e as the source) instead of unconditionally using
StorageError::Corruption so callers of load_metadata / the function that loads
the last target height can inspect the real cause.

Configuration used: defaults

Review profile: CHILL

Plan: Pro